Using mesh and associated survey points in the tracking process #2050
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
There was a problem hiding this comment.
Pull request overview
This pull request refactors the point structure and survey handling in the AliceVision codebase. It introduces support for mesh-based landmarks with locked coordinates, improves survey point tracking with residuals, and refactors the PointFetcher interface.
Changes:
- Refactored survey JSON format to support structured point data with multiple image observations per point
- Moved PointFetcher from sfm to sfmData namespace and extracted MeshPointFetcher into a separate header
- Replaced landmark "isPrecise" flag with "isLocked" flag and added point fetcher support for mesh-constrained landmarks
- Added residual tracking to survey points across all serialization formats (JSON, Alembic)
Reviewed changes
Copilot reviewed 29 out of 29 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| src/software/utils/main_sfmSurveyInjecting.cpp | Refactored to use new structured JSON format with boost::json tag_invoke for deserialization, computes residuals for survey points |
| src/software/utils/main_sfmPoseInjecting.cpp | Added lockPoses parameter to control whether injected poses can be refined |
| src/software/pipeline/main_sfmExpanding.cpp | Updated to use refactored MeshPointFetcher from sfmData namespace |
| src/software/pipeline/main_sfmBootstrapping.cpp | Updated to use refactored MeshPointFetcher and set reference views for mesh-based landmarks |
| src/aliceVision/sfmDataIO/sfmDataIO.hpp | Incremented version revision to 14 |
| src/aliceVision/sfmDataIO/jsonIO.cpp | Added serialization/deserialization support for landmark locking and survey point residuals |
| src/aliceVision/sfmDataIO/AlembicImporter.cpp | Added support for importing locked landmarks and survey point residuals |
| src/aliceVision/sfmDataIO/AlembicExporter.cpp | Added support for exporting locked landmarks and survey point residuals |
| src/aliceVision/sfmData/SurveyPoint.hpp | Added residual field to track projection errors |
| src/aliceVision/sfmData/PointFetcher.hpp | Moved from sfm namespace to sfmData, added pickPoint and getPointAndNormal methods |
| src/aliceVision/sfmData/MeshPointFetcher.hpp | New file extracting MeshPointFetcher as a concrete implementation of PointFetcher |
| src/aliceVision/sfmData/Landmark.i | Updated SWIG interface to ignore _isLocked instead of _isPrecise |
| src/aliceVision/sfmData/Landmark.hpp | Replaced isPrecise with isLocked, added point fetcher support for mesh-based landmarks |
| src/aliceVision/sfm/sfmFilters.hpp | Added eraseLandmarksWithMissingReferenceView function declaration |
| src/aliceVision/sfm/sfmFilters.cpp | Implemented eraseLandmarksWithMissingReferenceView to remove landmarks with invalid reference views |
| src/aliceVision/sfm/pipeline/expanding/SfmTriangulation.hpp | Updated to use sfmData::PointFetcher with shared_ptr instead of unique_ptr |
| src/aliceVision/sfm/pipeline/expanding/SfmTriangulation.cpp | Refactored mesh-based triangulation to store reference view and point fetcher with landmarks |
| src/aliceVision/sfm/pipeline/expanding/ExpansionProcess.cpp | Added filtering to remove mesh-based landmarks without valid point fetchers |
| src/aliceVision/sfm/pipeline/expanding/ExpansionChunk.hpp | Removed _enableMeshPrior flag (merged with depth prior) |
| src/aliceVision/sfm/pipeline/expanding/ExpansionChunk.cpp | Merged mesh and depth prior triangulation paths, improved logging |
| src/aliceVision/sfm/bundle/costfunctions/survey.hpp | New file implementing survey error cost function with residual-based tolerance |
| src/aliceVision/sfm/bundle/costfunctions/projectionMesh.hpp | New file implementing projection error for mesh-constrained landmarks |
| src/aliceVision/sfm/bundle/costfunctions/projection.hpp | Removed ProjectionSurveyErrorFunctor (replaced by SurveyErrorFunctor) |
| src/aliceVision/sfm/bundle/costfunctions/meshIntersect.hpp | New file implementing mesh intersection cost function with analytical Jacobian |
| src/aliceVision/sfm/bundle/BundleAdjustmentCeres.hpp | Added surveyInfos method declaration |
| src/aliceVision/sfm/bundle/BundleAdjustmentCeres.cpp | Implemented mesh-based landmark optimization, survey point filtering, and survey info logging |
| src/aliceVision/mesh/MeshIntersection.hpp | Added getPointAndNormal method for ray-based intersection |
| src/aliceVision/mesh/MeshIntersection.cpp | Implemented getPointAndNormal for direct ray intersection |
| meshroom/aliceVision/SfMPoseInjecting.py | Added lockPoses parameter to UI |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
servantftransperfect
force-pushed
the
dev/pointStructure
branch
from
07d3359 to
237b9d0
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 29 out of 29 changed files in this pull request and generated 23 comments.
Comments suppressed due to low confidence (1)
src/aliceVision/sfmDataIO/AlembicImporter.cpp:476
- The readSurveys function strictly requires sampleDataSize to be 7 (line 473 check). This will cause loading to fail for Alembic files created with older versions where sampleDataSize was 5. Consider adding backward compatibility by checking if the data size is divisible by 5 or 7, and handling both cases appropriately. For size 5, initialize residual to zero.
const int sampleDataSize = 7;
if (sampleId.size() * sampleDataSize != sampleData->size())
{
return false;
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+254
to
259
| // When using a mesh, we don't need to check minPoints | ||
| if (landmark.getPointFetcher() == nullptr) | ||
| { | ||
| continue; | ||
| } | ||
| } |
There was a problem hiding this comment.
The logic on lines 254-259 allows landmarks with a point fetcher to bypass the minimum observation count check (minPoints). While this makes sense for mesh-based landmarks which don't need parallax, the comment explaining this rationale should be clearer. Change the comment from "When using a mesh, we don't need to check minPoints" to something like "Mesh-based landmarks (with point fetcher) don't require minimum observation count as depth is constrained by the mesh".
servantftransperfect
force-pushed
the
dev/pointStructure
branch
from
237b9d0 to
0ed7118
Compare
servantftransperfect
changed the title
servantftransperfect
force-pushed
the
dev/pointStructure
branch
from
0ed7118 to
57f304f
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This pull request introduces several improvements and new features to the bundle adjustment and mesh intersection components, with a focus on supporting mesh-based constraints, handling survey points more robustly, and enabling pose locking for injected poses. The most notable changes include new cost functions for mesh intersection, enhanced survey point integration and reporting, and support for locking injected poses during refinement.
Mesh intersection and bundle adjustment enhancements:
getPointAndNormalto theMeshIntersectionclass and its header, allowing retrieval of a mesh point and normal given a 3D ray, which facilitates mesh-based constraints in optimization. [1] [2]CostMeshIntersectorinmeshIntersect.hpp, a new Ceres cost function for mesh intersection constraints, supporting analytic Jacobians for improved optimization accuracy.Survey point integration and reporting:
SurveyErrorFunctorcost function, and adds detailed reporting of survey reprojection errors after minimization via the newsurveyInfosmethod. [1] [2] [3] [4] [5]ProjectionSurveyErrorFunctorand replaced it with a more robust cost function for survey constraints.Landmark and pose refinement:
lockPosesboolean parameter toSfMPoseInjecting, allowing users to lock injected poses and prevent further refinement during bundle adjustment.These changes collectively improve the flexibility, robustness, and usability of mesh-based bundle adjustment and survey integration in the codebase.